Skip to content

Propagate Connection Closure to Client#64

Merged
JavaSaBr merged 12 commits intoJavaSaBr:developfrom
crazyrokr:propagate-connection-close
May 9, 2026
Merged

Propagate Connection Closure to Client#64
JavaSaBr merged 12 commits intoJavaSaBr:developfrom
crazyrokr:propagate-connection-close

Conversation

@crazyrokr
Copy link
Copy Markdown
Contributor

@crazyrokr crazyrokr commented May 6, 2026

This PR introduces the propagation of connection closures from the server to the client. Previously, clients might not have been correctly notified when a connection was closed by the server, potentially leading to stale connection states or unhandled exceptions in reactive streams.

Changes

  • Added ConnectionClosedException to explicitly handle connection closure scenarios.
  • Updated AbstractConnection to notify active FluxSink subscribers with a ConnectionClosedException when a connection is closed.
  • Modified AbstractNetworkPacketReader and AbstractSslNetworkPacketReader to correctly detect and report connection closures.
  • Added a new unit test ConnectionCloseTest to verify that the client correctly receives the ConnectionClosedException when a server connection is closed.

Comment thread rlib-network/src/main/java/javasabr/rlib/network/impl/AbstractConnection.java Outdated
Comment thread rlib-network/src/main/java/javasabr/rlib/network/impl/AbstractConnection.java Outdated
Comment thread rlib-network/src/main/java/javasabr/rlib/network/impl/AbstractConnection.java Outdated
@crazyrokr crazyrokr requested a review from JavaSaBr May 9, 2026 16:55
}

protected void notifySinksWithError(Throwable error) {
Array<FluxSink<?>> localActiveSinks = activeSinksOperations.getInReadLock(Array::copyOf);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you really want to allocate a full array for such reading?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so because sink.error(exc) in line 223 activates sink.onDispose() callback making write lock, so this situation causes dead lock. Any better ideas are welcome

https://github.com/crazyrokr/RLib/blob/35939cee7afbfc724d229ea08b733a7005393bdc/rlib-network/src/main/java/javasabr/rlib/network/impl/AbstractConnection.java#L148-L152

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves the rlib-network reactive API by ensuring server-side connection closures are propagated to client subscribers (via a dedicated ConnectionClosedException), reducing the risk of stale client connection state in reactive streams.

Changes:

  • Introduces ConnectionClosedException and emits it to active reactive subscribers when a connection closes.
  • Updates network packet readers to treat channel-closure exceptions as a signal to close the connection.
  • Adds unit tests (plus a shared await helper in rlib-common test fixtures) to validate closure propagation behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
rlib-network/src/test/java/javasabr/rlib/network/ConnectionCloseTest.java Adds tests for client notification on server close and for EOF handling when a client channel closes abruptly.
rlib-network/src/main/java/javasabr/rlib/network/packet/impl/AbstractSslNetworkPacketReader.java Treats EOF (-1) as an empty read to trigger connection closure handling in SSL mode.
rlib-network/src/main/java/javasabr/rlib/network/packet/impl/AbstractNetworkPacketReader.java Closes the connection on AsynchronousCloseException / ClosedChannelException to propagate closure to higher layers.
rlib-network/src/main/java/javasabr/rlib/network/impl/AbstractConnection.java Tracks active FluxSinks and notifies them with ConnectionClosedException during connection close.
rlib-network/src/main/java/javasabr/rlib/network/exception/ConnectionClosedException.java Adds a specific exception type for connection-closure signaling.
rlib-network/build.gradle Adds dependency on rlib-common test fixtures for shared test utilities.
rlib-common/src/testFixtures/java/javasabr/rlib/common/util/AwaitUtils.java Introduces a small polling helper used by network tests.
rlib-common/src/test/java/javasabr/rlib/common/util/AwaitUtilsTest.java Adds unit tests for the new await helper.

Comment thread rlib-network/src/main/java/javasabr/rlib/network/impl/AbstractConnection.java Outdated
Comment thread rlib-network/src/main/java/javasabr/rlib/network/impl/AbstractConnection.java Outdated
Comment thread rlib-network/src/test/java/javasabr/rlib/network/ConnectionCloseTest.java Outdated
Comment thread rlib-network/src/test/java/javasabr/rlib/network/ConnectionCloseTest.java Outdated
Comment thread rlib-network/src/test/java/javasabr/rlib/network/ConnectionCloseTest.java Outdated
crazyrokr and others added 3 commits May 9, 2026 19:38
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@crazyrokr crazyrokr requested a review from JavaSaBr May 9, 2026 18:05
Comment thread rlib-network/src/main/java/javasabr/rlib/network/impl/AbstractConnection.java Outdated
@JavaSaBr JavaSaBr merged commit 1866161 into JavaSaBr:develop May 9, 2026
1 of 2 checks passed
@crazyrokr crazyrokr deleted the propagate-connection-close branch May 9, 2026 18:19
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 9, 2026

Overall Project 54.38% -0.23% 🍏
Files changed 59.2%

File Coverage
AbstractConnection.java 75.69% -7.13% 🍏
AbstractNetworkPacketReader.java 61.98% -2.56%
AbstractSslNetworkPacketReader.java 49.7% 🍏
ConnectionClosedException.java 47.83% -52.17%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants